Skip to content

Conversation

@dimakozin
Copy link
Contributor

What's new

  • Saving previous global proxy state
  • Set previous state after tests

@dimakozin
Copy link
Contributor Author

up

const host = proxy ? `${proxy.ip}:${proxy.port}` : ':0';
const host = proxy?.ip ? `${proxy.ip}:${proxy.port}` : ':0';

if (realDevice && proxy) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also update the condition here to check if the proxy has a valid ip/port property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global proxies are defined in the "host:port" format, so I don’t think any additional checks are necessary.

src/plugin.ts Outdated
}
const realDevice = await isRealDevice(adb, deviceUDID);

this.previousGlobalProxy = await getGlobalProxyValue(adb, deviceUDID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing the previous proxy state in the class instance can lead to issues when tests run in parallel.

A better approach is to store the current proxy URL within the session’s proxy object, ensuring it remains unique to each device session.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed - previous proxy state saved in current proxy object

@P-Courteille
Copy link
Contributor

Hello,
I could be nice to use plugin argument instead of environment variable for the upstream proxy.
What do you think about it ?

Using the device farm plugin as example, we created a PR with plugin argument to use a custom certificate folder:
#76

The version is still 1.0.2 in this PR but the release is still stuck to 1.0.1:
image

Best regards,
Paul

@dimakozin
Copy link
Contributor Author

@sudharsan-selvaraj please review

@sudharsan-selvaraj sudharsan-selvaraj merged commit 1305816 into AppiumTestDistribution:master Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants